Cargo.toml (1)

30-30: Alloy 1.0.24 upgrade: verify MSRV and consider trimming features.

• Good move aligning the codebase to alloy 1.0.x. Please double-check that alloy 1.0.24 supports the declared MSRV (rust-version = "1.86") in this crate, and that CI runs with that toolchain.
• Using features = ["full"] can significantly increase compile times and dependency surface. If feasible, consider narrowing to only the used crates/features (e.g., primitives, rpc-types, consensus, providers, network). This will reduce build time and shrink the binary.

src/services/signer/evm/aws_kms_signer.rs (2)

116-121: Consider a fallback normalization for legacy branch only if needed.

Legacy transactions typically preserve v as 27/28 in external representations. If Signature::from_raw in alloy 1.0.x rejects 27/28, a guarded fallback keeps this branch robust while preserving 27/28 in the returned signature bytes.

-            let signature = Signature::from_raw(&signed_bytes)
-                .map_err(|e| SignerError::ConversionError(e.to_string()))?;
+            let signature = Signature::from_raw(&signed_bytes).or_else(|_| {
+                // Try with normalized v (0/1) if 27/28 was rejected
+                let mut b = signed_bytes.clone();
+                if b[64] == 27 {
+                    b[64] = 0;
+                } else if b[64] == 28 {
+                    b[64] = 1;
+                }
+                Signature::from_raw(&b)
+            }).map_err(|e| SignerError::ConversionError(e.to_string()))?;

Note: Keep the output bytes for legacy as-is (27/28) to match downstream expectations and existing tests.

---

316-320: Tests assert EIP-1559 v normalization.

The check for v in {0,1} for typed transactions is aligned with the change. Consider adding a small unit test for the legacy path verifying v in {27,28} to lock this behavior.

Do you want me to draft that test?

src/services/signer/evm/google_cloud_kms_signer.rs (1)

103-110: Optional: guarded fallback for legacy v if alloy rejects 27/28.

Mirror the AWS suggestion so both signers behave consistently under alloy 1.0.x.

-            let signature = Signature::from_raw(&signed_bytes)
-                .map_err(|e| SignerError::ConversionError(e.to_string()))?;
+            let signature = Signature::from_raw(&signed_bytes).or_else(|_| {
+                let mut b = signed_bytes.clone();
+                if b[64] == 27 { b[64] = 0; }
+                else if b[64] == 28 { b[64] = 1; }
+                Signature::from_raw(&b)
+            }).map_err(|e| SignerError::ConversionError(e.to_string()))?;

src/services/gas/evm_gas_price.rs (2)

443-446: Test: construct Block then convert to AnyRpcBlock — LGTM.

The pattern let mut block: Block = Block::default(); block.header.base_fee_per_gas = Some(...); Ok(AnyRpcBlock::from(block)) is clear and mirrors the production usage.

Consider extracting a small helper in the test module to avoid repeating this stub in multiple tests.

---

472-475: Repeat stub — consider factoring to a helper.

Same suggestion as above; reduces duplication and improves readability across tests.

Example helper outside the selected lines:

fn mock_block_with_base_fee(base_fee: u128) -> AnyRpcBlock {
    let mut block: Block = Block::default();
    block.header.base_fee_per_gas = Some(base_fee as u64);
    AnyRpcBlock::from(block)
}

src/models/rpc/evm/mod.rs (1)

25-26: Public aliases for BlockResponse and TransactionReceipt — good façade.

These type aliases provide a stable public surface while allowing internal library upgrades. Consider adding short doc comments to clarify they map to alloy “Any*” types and may evolve with dependency updates.

-pub type BlockResponse = AnyRpcBlock;
-pub type TransactionReceipt = AnyTransactionReceipt;
+/// Opaque EVM block response type (currently alloy::network::AnyRpcBlock).
+pub type BlockResponse = AnyRpcBlock;
+/// Opaque EVM transaction receipt type (currently alloy::network::AnyTransactionReceipt).
+pub type TransactionReceipt = AnyTransactionReceipt;

src/services/provider/evm/mod.rs (4)

23-30: Consider reordering fillers to minimize RPC round-trips and align with Alloy’s typical fill sequence.

Current order is Identity → Gas → BlobGas → Nonce → ChainId. In practice, resolving ChainId early can help subsequent fillers (Nonce/Gas) avoid redundant calls. A commonly used sequence is: ChainId → Nonce → Gas → BlobGas (wrapped with Identity).

This is optional and code will work as-is, but reordering can reduce latency on some providers.

---

221-243: Minor: avoid cloning the client when connecting the provider.

ClientBuilder::connect_client takes ownership; the local client isn’t reused. The .clone() is unnecessary.

Apply this diff:

-        let provider = ProviderBuilder::new()
-            .network::<AnyNetwork>()
-            .connect_client(client.clone());
+        let provider = ProviderBuilder::new()
+            .network::<AnyNetwork>()
+            .connect_client(client);

---

429-432: Use a more accurate error variant for TX hash parse failures.

TX hash parsing failures are being surfaced as InvalidAddress, which is misleading.

Apply this diff to avoid misclassification:

-        let parsed_tx_hash = tx_hash
-            .parse::<alloy::primitives::TxHash>()
-            .map_err(|e| ProviderError::InvalidAddress(e.to_string()))?;
+        let parsed_tx_hash = tx_hash
+            .parse::<alloy::primitives::TxHash>()
+            .map_err(|e| ProviderError::Other(format!("Invalid transaction hash: {}", e)))?;

---

461-472: Micro: avoid string allocation in raw_request_dyn.

You can pass the method as a borrowed Cow without cloning.

Apply this diff:

-            let method_clone = method.to_string();
-            let params_clone = params.clone();
+            let params_clone = params.clone();
@@
-                let result = provider
-                    .raw_request_dyn(std::borrow::Cow::Owned(method_clone), &params_raw)
+                let result = provider
+                    .raw_request_dyn(std::borrow::Cow::Borrowed(method), &params_raw)
                     .await
                     .map_err(ProviderError::from)?;

Also applies to: 474-477

src/domain/transaction/evm/status.rs (1)

593-620: Tests: avoid magic number for receipt r#type.

Using 0 for legacy type works but is brittle. Prefer the enum/constant provided by Alloy for readability and future-proofing (e.g., TxType::Legacy as u8 if available).

Optional, but recommended for clarity.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/services/signer/evm/aws_kms_signer.rs (2)

3-3: PrimitiveSignature → Signature change looks correct.

Import update matches alloy 1.0.x. No issues spotted here.

---

65-88: Normalize v before constructing Signature for EIP-1559

Some clients (e.g. Alloy 1.0.x) expect the recovery ID (v) to be 0 or 1 when parsing raw signatures for typed transactions. AWS KMS, however, returns v as 27 or 28. If you call Signature::from_raw(&signed_bytes) with a 27/28 recovery ID, it may fail, forcing you to mutate the bytes post-hoc. Normalizing v to 0/1 before invoking Signature::from_raw ensures the conversion succeeds and lets you drop the later adjustment.

• File: src/services/signer/evm/aws_kms_signer.rs
• Lines: 65–88

Apply this diff:

             // Ensure we have the right signature length
             if signed_bytes.len() != 65 {
                 return Err(SignerError::SigningError(format!(
                     "Invalid signature length from AWS KMS: expected 65 bytes, got {}",
                     signed_bytes.len()
                 )));
             }

-            // Construct primitive signature
-            let signature = Signature::from_raw(&signed_bytes)
-                .map_err(|e| SignerError::ConversionError(e.to_string()))?;
-
-            // Extract signature array bytes
-            let mut signature_bytes = signature.as_bytes();
+            // Normalize v for typed tx (expect 0/1) before constructing Signature
+            let mut sig_bytes = signed_bytes.clone();
+            match sig_bytes[64] {
+                27 => sig_bytes[64] = 0,
+                28 => sig_bytes[64] = 1,
+                0 | 1 => {}
+                other => {
+                    return Err(SignerError::SigningError(format!(
+                        "Invalid v value from AWS KMS for EIP-1559: {} (expected 0/1/27/28)",
+                        other
+                    )));
+                }
+            }
+            let signature = Signature::from_raw(&sig_bytes)
+                .map_err(|e| SignerError::ConversionError(e.to_string()))?;
+
+            // Extract signature bytes (already normalized to 0/1)
+            let signature_bytes = signature.as_bytes();

             // Construct a signed transaction
             let signed_tx = unsigned_tx.into_signed(signature);

-            // Adjust v value for EIP-1559 (27/28 -> 0/1)
-            if signature_bytes[64] == 27 {
-                signature_bytes[64] = 0;
-            } else if signature_bytes[64] == 28 {
-                signature_bytes[64] = 1;
-            }
+            // No-op: v already normalized to 0/1 for EIP-1559.

src/services/signer/evm/google_cloud_kms_signer.rs (2)

11-12: PrimitiveSignature → Signature migration acknowledged.

Import update aligns with alloy 1.0.x.

---

66-77: Ignore the pre‐normalization refactor as proposed
The Signature::from_raw(&bytes) constructor expects the final byte in “Electrum” notation (27 or 28). Normalizing to 0/1 before calling from_raw will cause a ConversionError. If you need to normalize the parity bit earlier, use one of these APIs instead:
• Signature::from_bytes_and_parity(&bytes[..64], parity_bool)
• Signature::from_raw(&bytes) followed by .with_parity(parity_bool)

As both AWS and GCP signers already correctly adjust v post‐hoc, you can leave the existing mutation in place or switch to one of the above methods—but do not feed 0/1 into from_raw.

Likely an incorrect or invalid review comment.

src/services/gas/evm_gas_price.rs (1)

313-316: Imports updated for AnyRpcBlock and Block/FeeHistory types — looks good.

This aligns test scaffolding with the new alloy RPC types.

src/models/rpc/evm/mod.rs (1)

1-1: New imports for AnyRpcBlock/AnyTransactionReceipt — OK.

Prepares the aliases below without leaking alloy types to all call-sites.

src/services/provider/evm/mod.rs (2)

313-327: Sanity check: Into conversions for estimate_gas/send_transaction/call look correct with Alloy 1.0+.

The switch to Into at call sites matches the updated provider API. No issues spotted here.

Also applies to: 336-351, 442-453

---

254-278: No changes required: ProviderError already implements From<String>

The repository contains the following implementation, satisfying the requirement for retry_rpc_call:

• src/services/provider/mod.rs:124–126

  // Add conversion from String to ProviderError
  impl From<String> for ProviderError {
      fn from(error: String) -> Self {
          ProviderError::Other(error)
      }
  }

src/domain/transaction/evm/status.rs (3)

5-5: Correct: import ReceiptResponse to bring status() into scope.

This is required for the trait-provided status() method on the nested receipt. Good catch.

---

63-66: Correct access to nested receipt status.

receipt.inner.status() aligns with the new AnyReceiptEnvelope-based structure introduced by Alloy 1.0+. This change prevents false positives when checking success.

---

741-763: Overall test harness updates are coherent with Alloy 1.0+ receipt model.

The switch to AnyReceiptEnvelope, ReceiptWithBloom, and Eip658Value is consistent and the status checks/confirmations logic remains intact. No issues here.

Also applies to: 765-895, 1035-1110, 1151-1173, 1199-1221, 1268-1375

src/services/provider/evm/mod.rs (7)

254-256: Generic retry closure now takes EvmProviderType: good, but prefer opaque provider if possible.

Using the concrete alias couples retry plumbing to filler composition. If you later change fillers, you’ll have to touch signatures across the file. Consider accepting a generic P: Provider<Network = AnyNetwork> (or an impl-trait/boxed provider) in the closure to reduce coupling.

Example (conceptual; outside the selected range):

• Change F to Fn(P) where P: Provider<Network = AnyNetwork> + Clone + Send + Sync + 'static.

---

317-325: Unnecessary into() on TransactionRequest for estimate_gas.

If provider.estimate_gas accepts the same TransactionRequest type from alloy::rpc::types, the extra into() is superfluous and can obscure type intent.

-                provider
-                    .estimate_gas(tx_req.into())
+                provider
+                    .estimate_gas(tx_req)

If alloy expects a different request type here, keep the conversion, but add a comment explaining the target type for clarity.

---

342-345: Same nit: avoid redundant into() for send_transaction if not required.

Keep it only if the method expects a different request flavor.

-                        .send_transaction(tx_req.into())
+                        .send_transaction(tx_req)

---

409-423: get_block_by_number: OK, but consider exposing a parameter for block tag if needed later.

Hard-wiring Latest is fine for now; if call sites start needing historical data, you’ll end up duplicating logic. Consider a variant that accepts BlockNumberOrTag.

---

429-433: Error variant for bad tx hash.

Mapping an invalid transaction hash to ProviderError::Other loses structure. If feasible, add a dedicated InvalidTxHash variant, or reuse an existing structured variant to ease upstream error handling and logging.

---

442-453: call_contract: same redundant into(), and consider passing a block tag.

• Nit: drop into() if not needed.
• Optional: expose an overload with a BlockNumberOrTag (default Latest) to support state overrides at specific blocks.

-                provider
-                    .call(tx_req.into())
+                provider
+                    .call(tx_req)

---

585-613: Unit test depends on external network; consider making it deterministic.

test_reqwest_error_conversion invokes https://www.openzeppelin.com/ with a 1ms timeout. This can fail for reasons other than timeout (DNS), making the test flaky. Prefer a deterministic approach (e.g., request a non-routable IP with a very short connect timeout, or construct a reqwest::Error via a mocked transport).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.